Repository navigation
Conversation
cbb330
added this pull request to stack #795
October 2, 2026 19:15
cbb330
removed this pull request from stack #795
October 2, 2026 23:22
cbb330
force-pushed
the
chbush/type2-rewrite-gate-new-snapshots-dpxm8
branch
from
October 2, 2026 23:22
e169411 to
3112335
Compare
cbb330
added this pull request to stack #798
October 2, 2026 23:24
6 tasks done
cbb330
added a commit
that referenced
this pull request
Oct 5, 2026
…ompiled constant (#793) ## Summary OpenHouse's Java client sends `User-Agent: openhouse-java-client/<version>` (#636), reading the version from the manifest of the jar that holds `WebClientFactory`. The runtime uber jars carry no `Implementation-Version`, and jars that re-bundle the client replace the manifest, so those clients send `openhouse-java-client/unknown`. In production over 24 hours, 35% of table loads and commits did. This change compiles the release into the client instead. The column-default client gate (#774) depends on it: it admits opted-in tables only for clients that report a release. Bottom of the column-default stack: #790, #774, #775, #681 and #794 build on it. ## Changes - [ ] Client-facing API Changes - [ ] Internal API Changes - [x] Bug Fixes - [ ] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [ ] Tests - `client/secureclient` generates `ClientVersion.VALUE` from `project.version` at build time; publishing sets it with `-Pversion`. Unversioned local builds still report `unknown`. - `WebClientFactory` reports that constant unless `setClientVersion`, or the catalog's `client-version` property, overrides it. A compiled constant survives shading, relocation and re-bundling. - The manifest lookup and the `secureclient` manifest stamp are removed. ## Testing Done - [ ] Manually Tested on local docker setup. Please include commands ran, and their output. - [ ] Added new tests for the changes made. - [ ] Updated existing tests to reflect the changes made. - [x] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [x] Some other form of testing like staging or soak time in production. Please explain. The failure only appears once the client is packaged, and CI builds without `-Pversion`, so a unit test can't catch it. `:client:secureclient:test` passes. I built the runtime uber jars with `-Pversion=0.5.999` and sent one request through `TablesApiClientFactory` to a local server that records the User-Agent: | Client classes loaded from | User-Agent | |---|---| | published `openhouse-java-runtime-0.5.503-uber.jar` | `openhouse-java-client/unknown` | | this branch's `openhouse-java-runtime` uber jar | `openhouse-java-client/0.5.999` | | this branch's `openhouse-spark-runtime_2.12` uber jar | `openhouse-java-client/0.5.999` | | the same classes unpacked, with no manifest | `openhouse-java-client/0.5.999` | | repacked into a jar whose manifest says `9.9.9` | `openhouse-java-client/0.5.999` | | with `setClientVersion("4.2.150")` | `openhouse-java-client/4.2.150` | # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [ ] Large PR broken into smaller PRs, and PR plan linked in the description. Clients that set `client-version` are unaffected. Clients that don't, and previously had a manifest version, now report the OpenHouse release they embed rather than the enclosing jar's version. li-openhouse sets `client-version` to its own runtime version in linkedin-multiproduct/li-openhouse#2420; merge that before li-openhouse picks up this release. <!-- 8kz0b -->
cbb330
force-pushed
the
chbush/type2-rewrite-gate-new-snapshots-dpxm8
branch
2 times, most recently
from
October 5, 2026 22:47
f96e658 to
b80ad36
Compare
cbb330
force-pushed
the
chbush/type2-rewrite-gate-new-snapshots-dpxm8
branch
2 times, most recently
from
October 6, 2026 17:12
a9d7a1b to
fcb3b8f
Compare
cbb330
force-pushed
the
chbush/type2-rewrite-gate-new-snapshots-dpxm8
branch
from
October 6, 2026 19:57
fcb3b8f to
9ebb74b
Compare
cbb330
force-pushed
the
chbush/type2-rewrite-gate-new-snapshots-dpxm8
branch
2 times, most recently
from
October 6, 2026 20:26
c74c2c2 to
12f69c4
Compare
Type 2 required the initial-default handshake only when a commit moved main to an overwrite or replace snapshot. An unaware client could still rewrite a stamped table on a named branch or as a staged WAP snapshot. Creating a branch or tag at an existing overwrite was rejected as a rewrite. The snapshots PUT carries the table's full snapshot list. Gate the overwrite and replace snapshots in it that the table has not persisted yet, whichever ref they are on. Persisted snapshot ids are read from the catalog only for a stamped table whose list contains such a snapshot; a failed read fails the commit as itself, since the request is not at fault. Fixes #693.
cbb330
force-pushed
the
chbush/type2-rewrite-gate-new-snapshots-dpxm8
branch
from
October 6, 2026 21:37
12f69c4 to
6ef9c8a
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #693. For a table with column defaults, Type 2 requires an overwrite or replace commit to send a matching
initial-default, so only default-aware clients can rewrite it. The check looked only at the snapshotmainpoints to. An unaware client could still rewrite the table on a named branch or as a staged WAP snapshot, and creating a branch or tag at an existing overwrite was rejected as a rewrite.The snapshots PUT already carries the table's full snapshot list. This change gates the overwrite and replace snapshots in that list that the table has not persisted yet, whichever ref they are on. It uses the existing request fields, so there is no API change. It compares snapshot ids, as the commit path already does to find new snapshots, and does not diff ref maps, which #693 rules out.
On #681, below the client gate (#774), which merges last. The code does not depend on the PRs below it; the stack keeps the column-default changes in one merge order.
Changes
Client-facing API Changes
Internal API Changes
Bug Fixes
New Features
Performance Improvements
Code Style
Refactoring
Documentation
Tests
Rewrite detection:
ReadBridgeStripProtectiontreats a commit as a rewrite when it is a replace, or whenjsonSnapshotscontains an overwrite or replace snapshot the table has not persisted. Snapshots already on the table are history, so ref-only commits (create a branch or tag, roll back to an existing snapshot) are not gated.Persisted snapshot ids: the new
OpenHouseInternalRepository.findSnapshotIds, wired inApiConfig, returns every snapshot id in the table's current metadata, including staged snapshots no ref points to. It is read only for a stamped, ramped table whose PUT lists an overwrite or replace snapshot, including older ones still in history. Those commits cost one extra metadata load; tables without column defaults pay nothing.Failure: if the lookup fails, the commit fails closed with the lookup's own exception. The request is not at fault, so it is not reported as a 400; the exception advice handles it like any other catalog failure.
initial-defaultinitial-defaultmaininitial-defaultTesting Done
Manually Tested on local docker setup. Please include commands ran, and their output.
Added new tests for the changes made.
Updated existing tests to reflect the changes made.
No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
Some other form of testing like staging or soak time in production. Please explain.
ReadBridgeStripProtectionTest: a branch overwrite next to a main append and a staged WAP overwrite are rejected; a branch and tag at a persisted overwrite, and a historical overwrite under a current append, are accepted; a failing lookup fails closed with its own exception.ReadBridgeColumnDefaultE2ETest.snapshotsPut_gatesAddedBranchRewritesButNotRefsAtPersistedRewritesdrives the snapshots endpoint through the real service, repository and catalog. A branch overwrite returns 400COLUMN_DEFAULT_REWRITE, a handshake overwrite onmaincommits, and a branch plus tag at that overwrite commit. Without the fix, onmainat5ee734d2, the branch overwrite returned 200.On this stack, with JDK 11:
:services:tables:testforreadbridge.*,ReadBridgeColumnDefaultE2ETest,SnapshotsControllerTest,mock.*,services.*,repository.*andconfig.*(686 tests) and:client:secureclient:test(20 tests) pass.Rebuilt below the client gate: the end-to-end test creates its table and PUTs snapshots without the gate's Spark-client headers.
:services:tables:test(923 tests) passes andspotlessCheckis clean.Restacked on [default values] Fail loudly when column defaults cannot be applied #790's typed failures (
6ef9c8a9): a failing lookup no longer becomesCOLUMN_DEFAULT_UNUSABLE(400).ReadBridgeStripProtectionTestpasses on this commit; at the top of the stack,:services:tables:test(924 tests) passes andspotlessCheckis clean.Additional Information
Stack
Stack #798, bottom to top:
true, with the policy override